Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add key action support in multi-edit mode #48

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

RibosomeK
Copy link

Hi, in this PR, I add two feature for multi-edit mode, as well as its chinese translation.
First is SetValue1-2 key actions support. Under multi-edit mode, value 1 is the first parameter on the left of the cursor, value 2 is the same but on the right.
The second one is two extra key actions for setting left and right parameter of current entry. But unfortunately, there is bug in getting newestcurrentIndex value, and I dont't know how to fix that. Can you help me on solving this problem?

…yAction to set currentEntry left and right border, along with chinese translation
@sdercolin
Copy link
Owner

sdercolin commented Nov 6, 2024

@RibosomeK sorry for the late response.
I rechecked our previous conversation and found some differences with your implementation here, so let me summarize my understanding here.

We are going to do 2 things here.

  1. Add a feature for multiple edit mode, to move the nearest lines (on left/right side) to the current cursor position. This should not depend on the current entry index, instead, only depends on the cursor position. Let me call it Snap Closest Left Parameter to Cursor (in Chinese maybe 吸附距离光标最近的左侧参数线). You are naming it SetCurrentEntryLeft, but it should not be the "current entry".
  • Actually, it's also useful for single edit mode, because we also have several lines on the screen at the same time
  1. Simply support setValueX for multiple edit mode and keep all existing behaviors. When selecting the target line to move, this feature doesn't depend on the cursor position, instead, depends on the "current entry index" and the "shortcutIndex" defined in the labeler configs. (e.g. nnsvs-singer-labeler already has them set)

Please check the following template.
I refactored some existing code and separate the two features we are working on, hope it would be more readable.
If you think it's okay, please update your impl using it. After that, let's talk about the detailed implementation, since it's quite complicated.

fun getUpdatedEntriesByKeyAction(
    action: KeyAction,
    appConf: AppConf,
    labelerConf: LabelerConf,
): Pair<List<EntryInPixel>, Int>? {
    val paramIndex = when (action) {
        KeyAction.SetValue1 -> 0
        KeyAction.SetValue2 -> 1
        KeyAction.SetValue3 -> 2
        KeyAction.SetValue4 -> 3
        KeyAction.SetValue5 -> 4
        KeyAction.SetValue6 -> 5
        KeyAction.SetValue7 -> 6
        KeyAction.SetValue8 -> 7
        KeyAction.SetValue9 -> 8
        KeyAction.SetValue10 -> 9
        else -> null
    }
    if (paramIndex != null) {
        return getUpdatedEntriesBySetParameterKeyAction(paramIndex, appConf, labelerConf)
    }

    val isCursorSnapRightSide = when (action) {
        KeyAction.SnapClosestLeftParameter -> false
        KeyAction.SnapClosestRightParameter -> true
        else -> null
    }
    if (isCursorAction) {
        return getUpdatedEntriesByCursorSnapKeyAction(isCursorSnapRightSide, appConf, labelerConf)
    }
    return null
}

private fun getUpdatedEntriesBySetParameterKeyAction(
    paramIndex: Int,
    appConf: AppConf,
    labelerConf: LabelerConf,
): Pair<List<EntryInPixel>, Int>? {

    // remove this -> Only used in single edit mode
    // remove this -> if (entries.size != 1) return null

    val fieldCount = this.labelerConf.fields.filter { it.shortcutIndex != null }.size
    val pointIndex = when {
        // TODO: Update the point index supporting multiple entries
        paramIndex == 0 -> MarkerCursorState.START_POINT_INDEX
        paramIndex == fieldCount + 1 -> MarkerCursorState.END_POINT_INDEX
        paramIndex <= fieldCount -> this.labelerConf.fields.indexOfFirst { it.shortcutIndex == paramIndex }
            .takeIf { it >= 0 } ?: return null
        else -> return null
    }
    val cursorPosition = cursorState.value.position ?: return null
    val lockDrag = appConf.editor.lockedSettingParameterWithCursor &&
        when (appConf.editor.lockedDrag) {
            AppConf.Editor.LockedDrag.UseLabeler -> {
                val lockedDragByBaseField =
                    labelerConf.lockedDrag.useDragBase &&
                        labelerConf.fields.getOrNull(pointIndex)?.dragBase == true
                val lockedDragByStart =
                    labelerConf.lockedDrag.useStart && pointIndex == MarkerCursorState.START_POINT_INDEX
                lockedDragByBaseField || lockedDragByStart
            }
            AppConf.Editor.LockedDrag.UseStart -> pointIndex == MarkerCursorState.START_POINT_INDEX
            else -> false
        }
    val entries = if (lockDrag) {
        getLockedDraggedEntries(pointIndex, cursorPosition, forcedDrag = false)
        // TODO: for multiple mode, we might need to return all `pointIndex`es that are actually affected by the locked drag
        // instead of the current `pointIndex`. so we may need to change the function signature, and update corresponding usages of the return value in `fun MarkerState.editEntryIfNeeded` in `Marker.kt`
    } else {
        getDraggedEntries(pointIndex, cursorPosition, forcedDrag = false)
    }
    return entries to pointIndex
}

private fun getUpdatedEntriesByCursorSnapKeyAction(
    isRightSide: Boolean,
    appConf: AppConf,
    labelerConf: LabelerConf,
): Pair<List<EntryInPixel>, Int>? {
    // TODO: implement this
    // can be similar to `getUpdatedEntriesBySetParameterKeyAction` but
    // 1. `pointIndex` is decided by the `cursorPosition`
    // 2. `lockDrag` is always `false`, because we don't want to move other parameters at the same time here. This is to say, `appConf.editor.lockedSettingParameterWithCursor` should not affect this new feature.
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants